docs: Updated README to Have Types #36
Conversation
Codecov Report
@@ Coverage Diff @@
## master #36 +/- ##
=====================================
Coverage 100% 100%
=====================================
Files 12 12
Lines 100 100
Branches 20 20
=====================================
Hits 100 100Continue to review full report at Codecov.
|
README.md
Outdated
| @@ -99,6 +99,10 @@ expect.extend({toBeInTheDOM, toHaveClass}) | |||
|
|
|||
| ### `toBeInTheDOM` | |||
|
|
|||
| ``` | |||
| toBeInTheDOM(container?: HTMLElement) | |||
There was a problem hiding this comment.
Since #29 we also support SGVElement, which for some reason is a class of its own. I guess we missed reflecting this change in the typescript types too, so maybe we could take the opportunity to reflect the change in that file as well as in the README.
@jgoz since you worked in #29 and also have worked on TypeScript stuff in this repo, can you confirm the typescript signatures need to be updated to reflect that they support sag elements as well?
There was a problem hiding this comment.
@gnapse That's a good question. I suppose it's possible that someone would be using an SVG element as a container, so maybe we should update the signature as follows (both in the readme and in definitions):
toBeInTheDOM(container?: HTMLElement | SVGElement)There was a problem hiding this comment.
Good to know. Are you up for it @jgoz? If not don't worry, maybe @smacpherson64 or I can do it later.
There was a problem hiding this comment.
For sure, I can add it to my list today.
README.md
Outdated
| @@ -152,6 +160,10 @@ expect(queryByTestId(container, 'not-empty')).not.toBeEmpty() | |||
|
|
|||
| ### `toContainElement` | |||
|
|
|||
| ``` | |||
| toContainElement(element: HTMLElement) | |||
There was a problem hiding this comment.
Same thing about the SVGElement support mentioned above.
|
BTW @smacpherson64 I'm not sure this qualifies as a |
|
About the |
smacpherson64
changed the title
|
If we need to update the typescript definitions, as I suspect, then it's a fix. Or we can leave it as is and make that fix separately. So maybe we can go forward with this as is. Feel free to do as you prefer. |
|
I think we should keep them separate and make a new issue to validate and fix the types. I am fairly sure we will need to add SVGElement as well: |
|
+1 to leave the fix for a separate PR. But now I have a suggestion for this one that I missed on my initial review. We should either make those code blocks syntax-aware of typescript (putting If we keep the current format (that is, the signature after the section title) then we must make that block syntax-aware of typescript though. |
|
I added typescript to the code blocks as I do not know how adding them to the headers will affect linking. |
README.md
Outdated
| ```typescript | ||
| toHaveClass(...classNames: string[]) | ||
| ``` | ||
|
|
||
| This allows you to check wether the given element has certain classes within its |
There was a problem hiding this comment.
Nitpick: wether should be whether (it's misspelled in a few spots)
README.md
Outdated
| @@ -99,6 +99,10 @@ expect.extend({toBeInTheDOM, toHaveClass}) | |||
|
|
|||
| ### `toBeInTheDOM` | |||
|
|
|||
| ``` | |||
| toBeInTheDOM(container?: HTMLElement) | |||
There was a problem hiding this comment.
@gnapse That's a good question. I suppose it's possible that someone would be using an SVG element as a container, so maybe we should update the signature as follows (both in the readme and in definitions):
toBeInTheDOM(container?: HTMLElement | SVGElement)As found by @jgoz fixing wether -> whether typo
|
Yes, good point regarding the links. They'd be updated, since the TOC is auto-generated, and it would all work, but it's ok this way anyway. Feel free to merge this, and remember to set the |
|
🎉 This PR is included in version 1.8.1 🎉 The release is available on:
Your semantic-release bot 📦🚀 |
What:
Updating readme to have types
Why:
To complete #33
How:
Added types (typescript types without return values)
Checklist:
Closes #33